ci: add caching w/ actions/cache@v3 #172
Conversation
This commit adds a caching step to the `build-windows`, `build` and `coverage` jobs of the GitHub actions CI configuration. We key based on: 1. Runner OS 2. Rust toolchain 3. Hash of Cargo.lock
|
You are essentially doing the same as swatinem's action if you look at the details, just with more manual configuration 🤷♂️ https://github.com/Swatinem/rust-cache?tab=readme-ov-file#cache-details If you are worried about supply-chain attacks from third-party actions, you could always pin by hash and let dependabot upgrade the action when there is a new release. |
True, but there's something to be said for an explicit and minimal configuration. It's also a language agnostic solution which may make future work like #34 easier. I don't feel strongly in either case. If you and Djc prefer the swatinem approach and can convince Est31 I'm happy to close this branch. |
Wouldn't you want separate caches for different languages? The hardest bit is cache invalidation and mixing caches only makes that harder. |
Fair point, phrased alternatively: we can learn actions/cache and use it for both languages as opposed to using rust-cache for rust and a separate action for other languages 🤷 |
|
Yeah personally I prefer using more "official" dependencies even if it means we have a few more lines to add and maintain. In other words, IMO the line overhead this PR is adding is not worth adding a dependency from a third party. The hacking risk is one of the smaller issues, I'm more generally concerned about additional components that can break and that we only have a poor understanding of. If I see the config file, I know precisely what it is doing. Not the same if there is a dependency. |
| ~/.cargo/git/db/ | ||
| target/ | ||
| $VCPKG_DEFAULT_BINARY_CACHE | ||
| key: ${{ runner.os }}-cargo-stable-${{ hashFiles('Cargo.lock') }} |
There was a problem hiding this comment.
Sorry to only notice this now but you probably want a separate restore-key here to use the cache even if Cargo.lock changes otherwise every dependency bump will completely invalidate the cache.
There was a problem hiding this comment.
Personally I'm fine with invalidating the cache a little bit too often rather than too rarely. Also, Cargo.lock is ideally only touched very rarely. I.e. I don't want to set up dependabot or run cargo update too regularly. It's just for testing and development purposes.
A possible alternative to #168. This commit adds a caching step to the
build-windows,buildandcoveragejobs of the GitHub actions CI configuration.We key based on:
This is largely 1:1 with the Cache action Rust Cargo docs, but for the Windows build we also include the
$VCPKG_DEFAULT_BINARY_CACHEto cache the result ofvcpkginstallation.